Update the default value of `unsafeDisableTooltip` to `false` and remove redundant props and wrappers #4702

broccolinisoup · 2024-06-27T05:08:28Z

Closes #2008
Related #4696

Note

We managed to complete the roll out at dotcom 🎉 (View the progress here (internal link only))

Changelog

Changed

The default value of unsafeDisableTooltip to false
Added unsafeDisableTooltip={true} a coupe of IconButton instances where it is not safe to enable tooltips.

Removed

Explicit Tooltip wrappers
unsafeDisableTooltip={false} prop since it is the default value now.

Rollout strategy

Patch release
Minor release
Major release; if selected, include a written rollout or migration plan
None; if selected, include a brief description as to why

Testing & Reviewing

Merge checklist

Added/updated tests
Added/updated documentation
Added/updated previews (Storybook)
Changes are SSR compatible
Tested in Chrome
Tested in Firefox
Tested in Safari
Tested in Edge
(GitHub staff only) Integration tests pass at github/github (Learn more about how to run integration tests)

…unncessary props and wrappers

changeset-bot · 2024-06-27T05:08:35Z

🦋 Changeset detected

Latest commit: fd6bd57

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@primer/react	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

github-actions · 2024-06-27T05:12:08Z

size-limit report 📦

Path	Size
packages/react/dist/browser.esm.js	91.05 KB (-0.08% 🔽)
packages/react/dist/browser.umd.js	91.38 KB (+0.04% 🔺)

broccolinisoup · 2024-06-28T05:34:15Z

packages/react/src/__tests__/Dialog.test.tsx

+      expect(tooltipEl).not.toHaveAttribute(':popover-open')
+    })
+
+    // second ESC closes the dialog and returns focus


This test is failing - Not sure how to address this. I am trying to await before pressing the second ESC but it doesn't seem to be working. Any suggestions, thoughts? 🤔

broccolinisoup · 2024-07-03T23:22:07Z

packages/react/src/__tests__/Dialog.test.tsx

@@ -140,7 +169,9 @@ describe('Dialog', () => {
  })

  it('Returns focus to returnFocusRef on escape', async () => {
-    const {getByTestId, queryByTestId} = HTMLRender(<Component />)


I had an issue with this existing component. It was supposed to go back to the return ref after pressing the ESC twice (First closing the tooltip, second closing the dialog) but for some reason it didn't work, even though I waited for the previous keyboard event to be completed. This test wasn't about the tooltip itself; it is about the Dialog so I updated to use a different dialog but let me know if I am misinterpreting the issue I encountered.

siddharthkp · 2024-07-08T08:57:04Z

packages/react/src/ButtonGroup/ButtonGroup.features.stories.tsx

@@ -11,7 +11,8 @@ export default {

 export const IconButtons = () => (
  <ButtonGroup>
-    <IconButton icon={PlusIcon} aria-label="Add" />
-    <IconButton icon={DashIcon} aria-label="Subtract" />
+    {/* We can remove these unsafe props after we resolve https://github.com/primer/react/issues/4129 */}


👍 Good to know!

siddharthkp · 2024-07-08T08:57:21Z

packages/react/src/Dialog.tsx

@@ -11,6 +11,7 @@ import type {ComponentProps} from './utils/types'
 import {useRefObjectAsForwardedRef} from './hooks/useRefObjectAsForwardedRef'
 import {XIcon} from '@primer/octicons-react'

+// Dialog v1


Classic! 😅

siddharthkp

Looks good! Does what it says on the box, good work!

Lets run some integration tests with this before we merge

primer-integration · 2024-07-09T00:46:48Z

👋 Hi from github/github! Your integration PR is ready: https://github.com/github/github/pull/332110

broccolinisoup · 2024-07-09T03:55:40Z

Integration tests pass https://github.com/github/github/pull/332110 and tested on RL, everything looks good 🎉

Update the default value of unsafeDisableTooltip to false and remove …

af79a10

…unncessary props and wrappers

broccolinisoup temporarily deployed to github-pages June 27, 2024 05:12 — with GitHub Actions Inactive

github-actions bot temporarily deployed to storybook-preview-4702 June 27, 2024 05:12 Inactive

test and lint fix

fdb2dae

broccolinisoup temporarily deployed to github-pages June 27, 2024 06:22 — with GitHub Actions Inactive

github-actions bot temporarily deployed to storybook-preview-4702 June 27, 2024 06:23 Inactive

Create clean-fireants-type.md

71bfea3

broccolinisoup temporarily deployed to github-pages June 27, 2024 06:49 — with GitHub Actions Inactive

github-actions bot temporarily deployed to storybook-preview-4702 June 27, 2024 06:50 Inactive

update actionbar e2e test selector

9186382

broccolinisoup temporarily deployed to github-pages June 28, 2024 02:57 — with GitHub Actions Inactive

github-actions bot temporarily deployed to storybook-preview-4702 June 28, 2024 02:58 Inactive

disable tooltips in button group stories

6f6482a

broccolinisoup temporarily deployed to github-pages June 28, 2024 03:37 — with GitHub Actions Inactive

github-actions bot temporarily deployed to storybook-preview-4702 June 28, 2024 03:37 Inactive

update Dialog tests

f5ffdc4

broccolinisoup changed the title ~~Update the default value of unsafeDisableTooltip to false and remove redundant props and wrappers~~ Update the default value of unsafeDisableTooltip to false and remove redundant props and wrappers Jun 28, 2024

broccolinisoup temporarily deployed to github-pages June 28, 2024 05:15 — with GitHub Actions Inactive

github-actions bot temporarily deployed to storybook-preview-4702 June 28, 2024 05:16 Inactive

fix linting

51ee65f

broccolinisoup marked this pull request as ready for review June 28, 2024 05:18

broccolinisoup requested a review from a team as a code owner June 28, 2024 05:18

broccolinisoup requested a review from TylerJDev June 28, 2024 05:18

broccolinisoup changed the title ~~Update the default value of unsafeDisableTooltip to false and remove redundant props and wrappers~~ Do not merge: Update the default value of unsafeDisableTooltip to false and remove redundant props and wrappers Jun 28, 2024

broccolinisoup temporarily deployed to github-pages June 28, 2024 05:20 — with GitHub Actions Inactive

github-actions bot temporarily deployed to storybook-preview-4702 June 28, 2024 05:21 Inactive

broccolinisoup requested review from siddharthkp and joshblack June 28, 2024 05:21

broccolinisoup commented Jun 28, 2024

View reviewed changes

joshblack removed their request for review July 2, 2024 15:25

broccolinisoup added 2 commits July 4, 2024 08:41

Merge branch 'main' into default-tooltip-icon-button

a659cc9

update tests

fd6bd57

broccolinisoup deployed to github-pages July 3, 2024 23:18 — with GitHub Actions View deployment

github-actions bot deployed to storybook-preview-4702 July 3, 2024 23:19 View deployment

broccolinisoup commented Jul 3, 2024

View reviewed changes

broccolinisoup requested a review from joshblack July 3, 2024 23:22

joshblack removed their request for review July 5, 2024 20:00

broccolinisoup changed the title ~~Do not merge: Update the default value of unsafeDisableTooltip to false and remove redundant props and wrappers~~ Update the default value of unsafeDisableTooltip to false and remove redundant props and wrappers Jul 8, 2024

siddharthkp reviewed Jul 8, 2024

View reviewed changes

siddharthkp approved these changes Jul 8, 2024

View reviewed changes

siddharthkp added react integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm labels Jul 8, 2024

broccolinisoup added this pull request to the merge queue Jul 9, 2024

Merged via the queue into main with commit 2536b49 Jul 9, 2024
36 checks passed

broccolinisoup deleted the default-tooltip-icon-button branch July 9, 2024 23:56

primer bot mentioned this pull request Jul 9, 2024

Release tracking #4728

Merged

snyk-io bot mentioned this pull request Aug 13, 2024

[Snyk] Upgrade @primer/react from 36.19.0 to 36.26.0 Playgirlkaybraz11/docs#2

Open

Mapuppy09 mentioned this pull request Aug 19, 2024

[Snyk] Upgrade @primer/react from 36.22.0 to 36.27.0 Mapuppy09/documentation#3

Merged

siddharthkp added integration-tests: passing Changes in this PR do NOT cause breaking changes in gh/gh and removed integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm labels Aug 20, 2024

sonukaloshiya mentioned this pull request Sep 4, 2024

[Snyk] Upgrade @primer/react from 36.23.0 to 36.27.0 sonukaloshiya/docs-1#3

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update the default value of `unsafeDisableTooltip` to `false` and remove redundant props and wrappers #4702

Update the default value of `unsafeDisableTooltip` to `false` and remove redundant props and wrappers #4702

broccolinisoup commented Jun 27, 2024 •

edited

Loading

changeset-bot bot commented Jun 27, 2024 •

edited

Loading

github-actions bot commented Jun 27, 2024 •

edited

Loading

broccolinisoup Jun 28, 2024

broccolinisoup Jul 3, 2024

siddharthkp Jul 8, 2024

siddharthkp Jul 8, 2024

siddharthkp left a comment •

edited

Loading

primer-integration bot commented Jul 9, 2024

broccolinisoup commented Jul 9, 2024

Update the default value of unsafeDisableTooltip to false and remove redundant props and wrappers #4702

Update the default value of unsafeDisableTooltip to false and remove redundant props and wrappers #4702

Conversation

broccolinisoup commented Jun 27, 2024 • edited Loading

Changelog

Changed

Removed

Rollout strategy

Testing & Reviewing

Merge checklist

changeset-bot bot commented Jun 27, 2024 • edited Loading

🦋 Changeset detected

github-actions bot commented Jun 27, 2024 • edited Loading

size-limit report 📦

broccolinisoup Jun 28, 2024

Choose a reason for hiding this comment

broccolinisoup Jul 3, 2024

Choose a reason for hiding this comment

siddharthkp Jul 8, 2024

Choose a reason for hiding this comment

siddharthkp Jul 8, 2024

Choose a reason for hiding this comment

siddharthkp left a comment • edited Loading

Choose a reason for hiding this comment

primer-integration bot commented Jul 9, 2024

broccolinisoup commented Jul 9, 2024

Update the default value of `unsafeDisableTooltip` to `false` and remove redundant props and wrappers #4702

Update the default value of `unsafeDisableTooltip` to `false` and remove redundant props and wrappers #4702

broccolinisoup commented Jun 27, 2024 •

edited

Loading

changeset-bot bot commented Jun 27, 2024 •

edited

Loading

github-actions bot commented Jun 27, 2024 •

edited

Loading

siddharthkp left a comment •

edited

Loading